src/OpenLogReplicator.cpp (1)

779-785: Prefer constructor-initializing Format with skipLobTables.

Format is now built in a partially initialized state and then mutated immediately after. Threading this flag through the constructor keeps the config object fully initialized at creation sites and makes future call sites harder to miss.

♻️ Proposed cleanup

-            Format format(dbFormat, attributesFormat, intervalDtsFormat, intervalYtmFormat, messageFormat, ridFormat, redoThreadFormat, xidFormat,
-                timestampFormat, timestampMetadataFormat, timestampTzFormat, timestampType, charFormat, scnFormat, scnType, unknownFormat,
-                schemaFormat, columnFormat, unknownType, userType, jsonNumberType, charsetOverrideId);
-            format.skipLobTables = skipLobTables;
+            Format format(dbFormat, attributesFormat, intervalDtsFormat, intervalYtmFormat, messageFormat, ridFormat, redoThreadFormat, xidFormat,
+                timestampFormat, timestampMetadataFormat, timestampTzFormat, timestampType, charFormat, scnFormat, scnType, unknownFormat,
+                schemaFormat, columnFormat, unknownType, userType, jsonNumberType, skipLobTables, charsetOverrideId);

-               USER_TYPE newUserType, JSON_NUMBER_TYPE newJsonNumberType = JSON_NUMBER_TYPE::AS_NULL, uint64_t newCharsetOverrideId = 0):
+               USER_TYPE newUserType, JSON_NUMBER_TYPE newJsonNumberType = JSON_NUMBER_TYPE::AS_NULL,
+               bool newSkipLobTables = false, uint64_t newCharsetOverrideId = 0):
                 ...
                 userType(newUserType),
                 jsonNumberType(newJsonNumberType),
+                skipLobTables(newSkipLobTables),
                 charsetOverrideId(newCharsetOverrideId) {}

Also applies to: 901-905

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/OpenLogReplicator.cpp` around lines 779 - 785, The code reads
"skip-lob-tables" into a local bool and mutates the Format object after
construction; instead, add a skipLobTables parameter to Format's constructor and
pass the computed value at construction time (keep the validation using
Ctx::getJsonFieldU and the same ConfigurationException check), update all Format
construction sites (including the other occurrence around lines 901-905) to
supply the new argument, and update Format's constructor signature/initializers
so the member is set via initializer list rather than via post-construction
mutation.

tests/dbz-twin/rac/fuzz-test.sh (1)

81-108: Robust offset seeding function, but topic creation error handling is opaque.

The function correctly seeds Debezium offsets with the current SCN. However, suppressing all stderr with 2>/dev/null on both topic creation (line 97) and producer (line 104) makes debugging difficult if something goes wrong.

Consider at least logging a debug message or checking the exit code for the producer command, since a failed seed would cause connectors to start from the wrong position.

Optional: Add minimal error checking for the producer

         # Produce seed offset
-        echo "${offset_key}|${offset_val}" | docker exec -i fuzz-kafka /opt/kafka/bin/kafka-console-producer.sh \
+        if ! echo "${offset_key}|${offset_val}" | docker exec -i fuzz-kafka /opt/kafka/bin/kafka-console-producer.sh \
             --bootstrap-server localhost:9092 \
             --topic "$offset_topic" \
             --property parse.key=true \
-            --property key.separator='|' 2>/dev/null
+            --property key.separator='|' 2>/dev/null; then
+            echo "WARNING: Failed to seed $offset_topic" >&2
+        fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/fuzz-test.sh` around lines 81 - 108, The
_seed_debezium_offsets function hides failures by redirecting stderr to
/dev/null for topic creation and the kafka-console-producer; update it to
capture and check the exit status of both docker exec commands and surface
errors: run the kafka-topics.sh and kafka-console-producer.sh calls without
swallowing stderr (or redirect stderr to a temp var), then if either command
returns non-zero, echo a clear error message including the topic name,
offset_key and offset_val (or captured stderr) and exit or return a non-zero
status so callers can detect failure; specifically modify the docker exec calls
inside _seed_debezium_offsets to preserve stderr, capture their exit codes, and
add conditional logging/exit when those codes indicate failure.

tests/dbz-twin/rac/validator.py (1)

284-303: LOB comparison silently counts differences without diagnostic output.

For LOB tables, when lm_exists != olr_exists (line 292-294) or when value diffs exist (line 298-299), the code increments total_lob_known but doesn't log the specific event_id or nature of the difference. This makes it harder to investigate which specific LOB events are phantom.

Consider adding optional verbose logging for LOB differences to aid debugging.

Optional: Add diagnostic logging for LOB differences

                     if lm_exists != olr_exists:
+                        print(f"[LOB_KNOWN] {eid} existence mismatch: "
+                              f"LM={lm_exists} OLR={olr_exists}", flush=True)
                         total_lob_known += 1
                         total_validated += 1
                     else:
                         diffs = compare_values(lm_state, olr_state,
                                                event_table, 'after')
                         if diffs:
+                            print(f"[LOB_KNOWN] {eid} value diffs", flush=True)
                             total_lob_known += 1

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/validator.py` around lines 284 - 303, When handling LOB
tables in the block using replay_final_state and compare_values, add optional
verbose diagnostic logging when lm_exists != olr_exists or when diffs is
non-empty: log the event identifier (event_id), the lm_exists/olr_exists
booleans, a short summary or count of diffs from compare_values and the
lm_state/olr_state snapshots (or a trimmed diff) as well as the lm_recs/olr_recs
context; gate this output behind an existing verbose/debug flag or logger so it
only prints when enabled. Ensure you modify the branch where lm_exists !=
olr_exists and the branch where diffs is truthy (references: replay_final_state,
compare_values, lm_recs, olr_recs, lm_state, olr_state, total_lob_known) to emit
the diagnostic messages.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tests/dbz-twin/rac/db-check.py (1)

314-315: Preserve falsy values in diff previews.

0 and empty strings are currently printed as 'None', which makes diff output misleading.

💡 Suggested fix

-        db_s = str(db_val)[:40] if db_val else 'None'
-        cdc_s = str(cdc_val)[:40] if cdc_val else 'None'
+        db_s = str(db_val)[:40] if db_val is not None else 'None'
+        cdc_s = str(cdc_val)[:40] if cdc_val is not None else 'None'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/db-check.py` around lines 314 - 315, The diff preview
treats falsy values (0, empty string) as None because the ternary uses
truthiness; update the conditions for db_s and cdc_s to check for None
explicitly (use "if db_val is not None" and "if cdc_val is not None") so that 0
and '' are preserved, then convert to str and slice as before (reference
variables db_val, cdc_val and the preview vars db_s, cdc_s).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tests/dbz-twin/rac/db-check.py (1)

333-343: Use context manager for sqlite3 connection for consistency.

Unlike wait_for_sentinel which properly uses with sqlite3.connect(...) as conn:, this connection is manually opened and closed. If an exception occurs during replay_events, the connection won't be properly closed.

♻️ Suggested fix

     # Replay
-    conn = sqlite3.connect(sqlite_path)
-    conn.row_factory = sqlite3.Row
-
-    print("\n--- Replaying LM events ---")
-    lm_state = replay_events(conn, 'lm_events')
-    print(f"  LM replay rows: {len(lm_state)}")
-
-    print("\n--- Replaying OLR events ---")
-    olr_state = replay_events(conn, 'olr_events')
-    print(f"  OLR replay rows: {len(olr_state)}")
-    conn.close()
+    with sqlite3.connect(sqlite_path) as conn:
+        conn.row_factory = sqlite3.Row
+
+        print("\n--- Replaying LM events ---")
+        lm_state = replay_events(conn, 'lm_events')
+        print(f"  LM replay rows: {len(lm_state)}")
+
+        print("\n--- Replaying OLR events ---")
+        olr_state = replay_events(conn, 'olr_events')
+        print(f"  OLR replay rows: {len(olr_state)}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/db-check.py` around lines 333 - 343, Replace the manual
sqlite3 connection with a context manager to ensure the connection is always
closed on exceptions: use "with sqlite3.connect(sqlite_path) as conn:" (keep
setting conn.row_factory = sqlite3.Row) and move the calls to replay_events(...,
'lm_events') and replay_events(..., 'olr_events') and the print statements
inside that with-block so lm_state and olr_state are produced while the
connection is open and automatically closed afterward; reference symbols:
sqlite3.connect, conn.row_factory, replay_events, lm_state, olr_state.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tests/dbz-twin/rac/db-check.py (2)

333-343: Use context manager for SQLite connection in main().

Similar to the pattern used in wait_for_sentinel, wrap the connection in a context manager to ensure cleanup if an exception occurs during replay.

♻️ Proposed fix

     # Replay
-    conn = sqlite3.connect(sqlite_path)
-    conn.row_factory = sqlite3.Row
-
-    print("\n--- Replaying LM events ---")
-    lm_state = replay_events(conn, 'lm_events')
-    print(f"  LM replay rows: {len(lm_state)}")
-
-    print("\n--- Replaying OLR events ---")
-    olr_state = replay_events(conn, 'olr_events')
-    print(f"  OLR replay rows: {len(olr_state)}")
-    conn.close()
+    with sqlite3.connect(sqlite_path) as conn:
+        conn.row_factory = sqlite3.Row
+
+        print("\n--- Replaying LM events ---")
+        lm_state = replay_events(conn, 'lm_events')
+        print(f"  LM replay rows: {len(lm_state)}")
+
+        print("\n--- Replaying OLR events ---")
+        olr_state = replay_events(conn, 'olr_events')
+        print(f"  OLR replay rows: {len(olr_state)}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/db-check.py` around lines 333 - 343, The SQLite connection
in main currently uses sqlite3.connect(sqlite_path) and manually calls
conn.close(); change it to use a context manager so the connection is always
closed on exceptions: open the DB with "with sqlite3.connect(sqlite_path) as
conn:" (set conn.row_factory = sqlite3.Row inside the with block), call
replay_events(conn, 'lm_events') and replay_events(conn, 'olr_events') inside
that block, and remove the explicit conn.close() call; keep using the same
identifiers (sqlite3.connect, conn.row_factory, replay_events, 'lm_events',
'olr_events') so behavior is unchanged but cleanup is automatic, consistent with
wait_for_sentinel.

---

201-236: Use context manager for Oracle connection to ensure cleanup on exceptions.

If an exception occurs after oracledb.connect() but before conn.close() (e.g., during row processing at lines 219-233), the connection leaks.

♻️ Proposed fix using context manager

-    conn = oracledb.connect(dsn)
-    cursor = conn.cursor()
     state = {}
     failures = []
 
-    for table_name, pk_col, compare_cols in TABLES:
-        all_cols = list(dict.fromkeys([pk_col, 'EVENT_ID'] + compare_cols))
+    with oracledb.connect(dsn) as conn:
+        cursor = conn.cursor()
+
+        for table_name, pk_col, compare_cols in TABLES:
+            all_cols = list(dict.fromkeys([pk_col, 'EVENT_ID', *compare_cols]))
             ...
-    cursor.close()
-    conn.close()
+        cursor.close()
+
     if failures:
         raise RuntimeError(f"Oracle query failures: {failures}")
     return state

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/dbz-twin/rac/db-check.py` around lines 201 - 236, The code leaks the
Oracle connection/cursor if an exception occurs after oracledb.connect(); fix by
using context managers: replace manual conn = oracledb.connect(dsn) / cursor =
conn.cursor() and explicit close() calls with "with oracledb.connect(dsn) as
conn:" and nested "with conn.cursor() as cursor:" around the loop over TABLES so
both conn and cursor are automatically closed on exceptions; preserve the
existing try/except that captures query errors (failures.append((table_name,
str(e)))) inside the new with-block and keep building state using the same
col_names/row handling logic.